feat(sftp): DDS integration into the rover (DHEI-20905) by valenok4000 · Pull Request #445 · telekom/controlplane

valenok4000 · 2026-06-29T12:31:40Z

Make possible the DDS configuration from the Rover.

Copilot

Pull request overview

This PR extends the Rover-facing OpenAPI contract to support configuring a DDS-based file-transfer backend (file exposures/subscriptions), regenerates the Go API types from the updated spec, and introduces a feature flag to keep the file domain disabled until the downstream domains are available.

Changes:

Added FileExposure, FileSubscription, and PublicKey schemas to the OpenAPI spec and wired them into the Exposure/Subscription discriminator unions.
Regenerated oapi-codegen Go types to include the new file exposure/subscription variants and union helper methods.
Added a new FeatureFile feature flag (default disabled) to gate upcoming file/DDS domain functionality.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File	Description
rover-server/internal/api/server.gen.go	Regenerated OpenAPI Go models/unions to include file exposure/subscription variants.
rover-server/api/openapi.yaml	Adds file exposure/subscription schemas and updates discriminator mappings for request unions.
common/pkg/config/feature.go	Adds a new `FeatureFile` flag (disabled by default) for the file/DDS domain rollout.

Files not reviewed (1)

rover-server/internal/api/server.gen.go: Generated file

Comments suppressed due to low confidence (1)

rover-server/internal/api/server.gen.go:4

The generated server stub no longer contains the SPDX license header. This can break REUSE/SPDX compliance checks for code files; if this file is intentionally generated, the generator/template should still emit the header (or the repo should explicitly exclude this path).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

    Exposure:
      type: object
      discriminator:
        propertyName: type
        mapping:
          api: '#/components/schemas/ApiExposure'
          event: '#/components/schemas/EventExposure'
+          file: '#/components/schemas/FileExposure'
      oneOf:
        - $ref: '#/components/schemas/ApiExposure'
        - $ref: '#/components/schemas/EventExposure'
+        - $ref: '#/components/schemas/FileExposure'


    Subscription:
      type: object
      discriminator:
        propertyName: type
        mapping:
          api: '#/components/schemas/ApiSubscription'
          event: '#/components/schemas/EventSubscription'
+          file: '#/components/schemas/FileSubscription'
      oneOf:
        - $ref: '#/components/schemas/ApiSubscription'
        - $ref: '#/components/schemas/EventSubscription'
+        - $ref: '#/components/schemas/FileSubscription'


…nd tests

add mapping for exp/subs into the rover

change logic on client creation

ron96g

I understand that this is not done yet but here is a first review of the current code.

ron96g · 2026-07-06T06:56:49Z

+type FileExposureSpec struct {
+	// Approval configures how subscriptions to this file type are approved.
+	// +kubebuilder:default=Simple
+	Approval ApprovalStrategy `json:"approval,omitempty"`


Must be inside of an Object with strategy: ApprovalStrategy and TrustedTeams: []string, see API and Event

ron96g · 2026-07-06T06:57:36Z

+	// FileTypeRef references the file-domain FileType this exposure belongs to.
+	// It must live in the same namespace as this FileExposure.
+	// +kubebuilder:validation:Required
+	FileTypeRef ctypes.ObjectRef `json:"fileTypeRef"`


The ref on this layer is done via the string-value of fileType and not via ObjectRef. This is needed as we might need to switch the active FileType (if migrated)

ron96g · 2026-07-06T06:58:27Z

+	// ZoneServiceConfigRef references the zone-scoped service configuration
+	// (provided by the SFTP/DDS domain) used to reach the backend for this exposure.
+	// +optional
+	ZoneServiceConfigRef *ctypes.ObjectRef `json:"zoneServiceConfigRef,omitempty"`


On this Layer we just pass the Zone-Ref and the domain itself must resolve it to its config, so Zone ctypes.ObjectRef

ron96g · 2026-07-06T06:59:19Z

+type FileSubscriptionSpec struct {
+	// FileTypeRef references the file-domain FileType this subscription belongs to.
+	// +kubebuilder:validation:Required
+	FileTypeRef ctypes.ObjectRef `json:"fileTypeRef"`


Same as for the Exposure. Ref via string-value and not via Ref

ron96g · 2026-07-06T06:59:52Z

+	Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
+
+	// Active indicates whether this subscription has been provisioned.
+	Active bool `json:"active,omitempty"`


Active flag is only for Exposure and Type, not subscription. There we use conditions

ron96g · 2026-07-06T07:01:03Z

+// file domain from a rover-domain FileSpecification (1:1) and is the canonical
+// resource that FileExposure (1:1) and FileSubscription (1:n) reference via their
+// fileTypeRef (mirrors event.EventType).
+type FileTypeSpec struct {


The FileType string is missing (similiar to eventType and basePath). Do we also need to add a specification field with the file-manager ref?

add Type as it's done for event. add specification

ron96g · 2026-07-06T07:02:09Z

+
+	// ExposureRef references the file-domain FileExposure created for this file type (1:1).
+	// +optional
+	ExposureRef *ctypes.ObjectRef `json:"exposureRef,omitempty"`


Both ExposureRef and SubscriptionRefs break our pattern and hierarchy: Type > Exposure > Subscription.
The FileType does not know about its Exposure and Subscription

true. deleted

ron96g · 2026-07-06T07:02:38Z

+	// +optional
+	Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
+
+	// Active indicates whether this FileType has been provisioned.


Comment is a bit misleading. This active-flag is used to indicate that THIS CR is the active one (singleton)

ron96g · 2026-07-06T07:03:10Z

          type: array
          items:
            $ref: '#/components/schemas/EventScope'
+    FileExposure:


This is aligned with the current Openapi-Spec?

add / change file/rover api add feature config, handler controller wh for file FileSpecification in progress

ron96g

See comments. The whole handler-logic in the file domain is still missing, correct?

ron96g · 2026-07-09T09:00:47Z

+	// PublicKeys are the SSH public keys registered for the provider's SFTP user.
+	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:MinItems=1
+	PublicKeys []PublicKey `json:"publicKeys"`


Why is this a root-level field? Did we not say that we want sub-objects like

sftp: publicKeys: []PublicKey cloudwalker: some: stuff

**Cloudwalker is just a placeholder here :D

spec: approval: SIMPLE fileType: ... sftp: publicKeys: - key: ssh-rsa AAAAB3NzaC1... label: coran-int-dds visibility: ENTERPRISE zone: name: ... namespace: ...

ron96g · 2026-07-09T09:01:08Z

+	// PublicKeys are the SSH public keys registered for the consumer's SFTP user.
+	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:MinItems=1
+	PublicKeys []PublicKey `json:"publicKeys"`


Same as Exposure

adjusted to the structure (from design)

spec: sftp: clientId: <subscription application....> publicKeys: - key: ssh-rsa AAAAB3NzaC1... label: coran-int-dds - key: ssh-rsa AAAAB3NzaC1... label: coran-int-dds-2

ron96g · 2026-07-09T09:02:08Z


 		out.Event = mapEventExposure(eventExp)

+	case "file":


Is there not "out" mapping for this? What about applicationinfo?

I add out mapping.
applicationInfo???

ron96g · 2026-07-09T09:02:58Z

 import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega/gstruct"


why is this here?

deleted (revert to initial state, it was (over)generated by AI

ron96g · 2026-07-09T09:04:07Z

+// It will be called by the file-domain handler
+// (rover/internal/handler/rover/file, added in DHEI-20903) when logging/serialising
+// the selected variant while creating the file-domain CRD.
+func (v FileVariant) String() string {


We dont need this. We no longer support multiple variants

that's good, will delete it

ron96g · 2026-07-09T09:14:20Z

+
+		fileSubscription.Spec = filev1.FileSubscriptionSpec{
+			FileType:   sub.FileType,
+			PublicKeys: mapPublicKeys(sub.PublicKeys),


I would add zone here as well just to keep it similiar. Even though it not used right now, correct?

Also I think you will need the Requestor values for the Approval-Handling

I would add zone here as well just to keep it similiar. Even though it not used right now, correct?

Refering to the internal documentation and design https://gitlab.devops.telekom.de/dhei/products/controlplane/docs/dds-file-integration
we don't have this zone for specification.

I would add zone here as well just to keep it similiar. Even though it not used right now, correct?

We keep it like this in design, not sure that similarity must be our point of truth.. will see

ron96g · 2026-07-09T09:15:55Z

+// MakeName generates a deterministic resource name for a file exposure or
+// subscription: "<fileType>--<owner>" (spec_dcp naming), normalized.
+func MakeName(fileType, ownerName string) string {
+	return filev1.MakeFileTypeName(fileType) + "--" + ownerName


There are functions to Normalize these values, see labelutil

ron96g · 2026-07-09T09:17:02Z

+	// The file type identifier lives in metadata.name (no spec.type field in the
+	// internal CRD, per spec_dcp); the client-side name==type rule is enforced by
+	// rover-server / roverctl.
+	if st := filespecification.Spec.StorageType; st != "" && st != roverv1.FileStorageTypeSFTP {


Not needed as done via CEL. Also I would remove it as not needed for now. We only have sftp and no plans to add more

ron96g · 2026-07-09T09:17:24Z

+
+// +kubebuilder:webhook:path=/validate-rover-cp-ei-telekom-de-v1-filespecification,mutating=false,failurePolicy=fail,sideEffects=None,groups=rover.cp.ei.telekom.de,resources=filespecifications,verbs=create;update,versions=v1,name=vfilespecification-v1.kb.io,admissionReviewVersions=v1
+
+type FileSpecificationCustomValidator struct {


I think the entire webhook is not needed

ron96g · 2026-07-09T09:18:26Z

+	exposesFiles := slices.ContainsFunc(rover.Spec.Exposures, func(exp roverv1.Exposure) bool {
+		return exp.Type() == roverv1.TypeFile
+	})
+	if (subscribesToFiles || exposesFiles) && !roverv1.IsFileTypeZoneSupported(rover.Spec.Zone) {


As other comments states. Should be done via feature-flag logic in admin and not hardcoded in rover.

… and fix the rover mapper out remove zone_supported flag and webhook as it's covered by featureEnabled flag add rbac for file domain

…struct" lib as it was added by AI add crypto lib and ssh pub key validation

rename func

move func desc to the original func

# Conflicts: # common/pkg/config/feature.go # rover-ctl/pkg/handlers/v0/rover.go # rover-ctl/pkg/handlers/v0/rover_test.go # rover-server/api/openapi.yaml # rover-server/internal/api/server.gen.go # rover-server/internal/mapper/rover/in/__snapshots__/exposure_test.snap # rover-server/internal/mapper/rover/in/__snapshots__/rover_test.snap # rover-server/internal/mapper/rover/in/__snapshots__/subscription_test.snap # rover-server/internal/mapper/rover/in/exposure.go # rover-server/internal/mapper/rover/in/subscription.go # rover-server/internal/mapper/rover/out/exposure.go # rover/api/v1/rover_types.go # rover/api/v1/zz_generated.deepcopy.go # rover/cmd/main.go # rover/config/crd/bases/rover.cp.ei.telekom.de_rovers.yaml # rover/config/rbac/role.yaml # rover/go.mod # rover/internal/controller/rover_controller.go # rover/internal/handler/rover/application/application.go # rover/internal/handler/rover/handler.go # rover/internal/webhook/v1/rover_webhook.go

# Conflicts: # rover-server/internal/mapper/rover/in/__snapshots__/exposure_test.snap # rover-server/internal/mapper/rover/in/__snapshots__/rover_test.snap # rover-server/internal/mapper/rover/in/__snapshots__/subscription_test.snap # rover-server/internal/mapper/rover/out/exposure.go # rover/api/v1/rover_types.go # rover/api/v1/zz_generated.deepcopy.go # rover/internal/handler/rover/handler.go

fix some issues

# Conflicts: # rover-server/internal/api/server.gen.go

feat(dhei-20905): api and gen for dds

9a2b3f6

Copilot AI review requested due to automatic review settings June 29, 2026 12:31

Copilot started reviewing on behalf of valenok4000 June 29, 2026 12:32 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

valenok4000 assigned lukas016 and ppuha Jun 29, 2026

valenok4000 added 7 commits June 30, 2026 10:05

feat(dhei-20905): add info for exposure and subs

9cc7467

feat(dhei-20905): add copyright header

5e4f0ad

feat(dhei-20905): delete memeories dds from OS

c33ed13

feat(dhei-20905): add mapping for FileExposure and FileSubscription a…

360fc12

…nd tests

feat(dhei-20905): add webhook and validation for file types

a5ba5da

add mapping for exp/subs into the rover

feat(dhei-20905): change config/feature comment

9361abd

feat(dhei-20905): file domain exp/subs handler

4fa6383

change logic on client creation

ron96g requested changes Jul 6, 2026

View reviewed changes

feat(dhei-20905): fix comments of PR

cfc8343

add / change file/rover api add feature config, handler controller wh for file FileSpecification in progress

valenok4000 changed the title ~~DDS integration into the rover (DHEI-20905)~~ feat(sftp): DDS integration into the rover (DHEI-20905) Jul 7, 2026

feat(dhei-20905): add tests and info files

a8ce3f6

ron96g requested changes Jul 9, 2026

View reviewed changes

valenok4000 added 12 commits July 17, 2026 17:02

feat(dhei-20905): move public keys to the sftp lvl of the file domain…

b79a651

… and fix the rover mapper out remove zone_supported flag and webhook as it's covered by featureEnabled flag add rbac for file domain

feat(sftp integration): dhei-20905 delete . "github.com/onsi/gomega/g…

924b7cc

…struct" lib as it was added by AI add crypto lib and ssh pub key validation

feat(sftp integration): fix hardcoded approval strategy

0117241

feat(sftp integration): delete the variant proccessing

9c10b72

rename func

feat(sftp integration): delete the variant proccessing

7bab199

rename func

owner name normalization

bb0e344

move func desc to the original func

backmerge from main

d0c6eb3

fix the conditional block

56d19fb

add tests for coverage

1e098e6

fix some issues

add tests

661c1a8

valenok4000 added 13 commits August 4, 2026 16:22

add tests

d02a25f

feat(rover): lint fix

85e967d

Merge branch 'main' into feat/ar-dds-integration-dhei-20905

a919ca5

# Conflicts: # rover-server/internal/api/server.gen.go

feat: fix failing tests

7bac2f9

feat: sum forgotten

ee6e844

feat: add test file add header

dbd17cd

feat: add test file

c805a59

feat: enable file feature

e67fe8c

Merge branch 'main' into feat/ar-dds-integration-dhei-20905

c3a0d8a

feat: add/modify tests

0c2cd81

feat: back reuse compl

eb28a5f

feat: changed rbac group name

8b801e8

feat: restore deleted files to make compilation success

e2571cf


		// +kubebuilder:webhook:path=/validate-rover-cp-ei-telekom-de-v1-filespecification,mutating=false,failurePolicy=fail,sideEffects=None,groups=rover.cp.ei.telekom.de,resources=filespecifications,verbs=create;update,versions=v1,name=vfilespecification-v1.kb.io,admissionReviewVersions=v1

		type FileSpecificationCustomValidator struct {

Uh oh!

Conversation

valenok4000 commented Jun 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

ron96g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ron96g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valenok4000 Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valenok4000 Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valenok4000 Jul 17, 2026 •

edited

Loading

valenok4000 Jul 17, 2026 •

edited

Loading